Skip to content

Conversation

patrickreiffenstein
Copy link
Contributor

@patrickreiffenstein patrickreiffenstein commented Jun 12, 2024

Fixes #357.

@krestenlaust krestenlaust changed the title Duplicate query removal when buying Optimize multibuy database queries Jun 13, 2024
Copy link
Member

@krestenlaust krestenlaust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to run it locally to check things over, and verify that we have tests in place

@krestenlaust krestenlaust self-requested a review June 13, 2024 07:51
@krestenlaust
Copy link
Member

krestenlaust commented Aug 5, 2024

I'll try to review this before summer ends. Probably after the current update has been finished

@JakobTopholt
Copy link
Contributor

@Mast3rwaf1z Vil du ikke være en king og review den her så Kresten kan få lidt fritid?

@krestenlaust
Copy link
Member

♥️ jeg vil dog stadig gerne merge den

@Mast3rwaf1z
Copy link
Member

@Mast3rwaf1z Vil du ikke være en king og review den her så Kresten kan få lidt fritid?

Tager lige et kig senere, var lige til forelæsning

Copy link
Member

@Mast3rwaf1z Mast3rwaf1z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small things, i think improves readability of the code, and removed some redundant constructor calls?

also i believe by using _ in a for loop prevents memory being allocated to the variable. really insignificant but hey, its good to have imo

@krestenlaust krestenlaust marked this pull request as draft September 24, 2024 21:50
@krestenlaust krestenlaust removed their request for review September 24, 2024 21:50
Added the suggestions from Kresten and Thomas, which seems to be some good changes

Co-authored-by: Kresten Laust <[email protected]>
Co-authored-by: Thomas Jensen <[email protected]>
@patrickreiffenstein patrickreiffenstein marked this pull request as ready for review January 11, 2025 20:17
Copy link
Member

@Mast3rwaf1z Mast3rwaf1z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, minor annoying naming issue

@patrickreiffenstein patrickreiffenstein removed the request for review from krestenlaust September 22, 2025 14:29
@patrickreiffenstein patrickreiffenstein marked this pull request as draft September 22, 2025 15:52
@patrickreiffenstein
Copy link
Contributor Author

My fork is broken and I don't know why it acts that way, so these changes are moved to #589.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Poor performance on high amount of products bought (even with multibuy)

4 participants